RSDK-2478: Update encoder driver methods #2161
Conversation
viambot
added
the
safe to test
viambot
added
safe to test
This comment was marked as duplicate.
This comment was marked as duplicate.
|
This PR is not ready yet I will update with a comment when it is. Realized I forgot to add a function. Sorry that you already reviewed @npmenard, but yes I did test on hardware and will retest when I'm actually done! |
viambot
added
safe to test
|
Should be ready now but won't pass the checks until api changes are merged |
|
sorry, I clicked approve on the wrong PR, I'm still reviewing |
viambot
added
safe to test
| } | ||
| ) | ||
|
|
||
| type mock struct { |
There was a problem hiding this comment.
[if-minor] I think we can use inject.Encoder for this purpose; setupInjectRobot can return a state object with these fields that gets mutated by the injected methods via closure. Then you can check the values on the state in the tests. Helps with codebase consistency if everything is using inject, but if it turns out to be too much trouble don't force it
There was a problem hiding this comment.
leaving this as is for now
viambot
added
safe to test
viambot
added
safe to test
|
@gvaradarajan @randhid I think I covered everything, but it resulted in a lot more changes so let me know if anything else comes up! |
| } | ||
| if !props[encoder.TicksCountSupported] { | ||
| return nil, | ||
| errors.New("cannot create an encoded motor without an encoder that has propery TicksCountSupported") |
There was a problem hiding this comment.
[nit] typo + maybe add
| errors.New("cannot create an encoded motor without an encoder that has propery TicksCountSupported") | |
| errors.New("encoder type is %v need an encoder that supports Ticks for an encoded motor", props) |
viambot
added
safe to test
viambot
added
safe to test
viambot
added
safe to test
This comment was marked as off-topic.
This comment was marked as off-topic.
This covers RSDK-2477 (Add client and server files), RSDK-2478 (update driver methods), and RSDK-2479 (add encoder to registry), to make separate driver packages for single, incremental, and AMS encoders.